Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

doc: killSignal option accepts integer values #10424

Closed
wants to merge 1 commit into from
Closed

doc: killSignal option accepts integer values #10424

wants to merge 1 commit into from

Conversation

thefourtheye
Copy link
Contributor

Checklist
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

doc child_process

Description of change

killSignal option accepts the signal name or signal number as well.

@thefourtheye
doc: killSignal option accepts integer values
33af045 
`killSignal` option accepts the signal name or signal number as well.
@nodejs-github-bot nodejs-github-bot added child_process Issues and PRs related to the child_process subsystem. doc Issues and PRs related to the documentations. lts-watch-v6.x labels Dec 23, 2016
@thefourtheye thefourtheye mentioned this pull request Dec 23, 2016
Merged
3 tasks
@sam-github
Copy link
Contributor

Is this behaviour covered by tests?

@thefourtheye
Copy link
Contributor Author

@sam-github There are few cases in test/parallel/test-child-process-spawnsync-validation-errors.js, but #10423 has enough cases I believe.

@sam-github
Copy link
Contributor

Are you saying #10424 is documentation for #10423? If so, merge the docs into the PR that introduces the API being documented.

@thefourtheye
Copy link
Contributor Author

@sam-github This behaviour is already there. #10423 improves the validation.

@sam-github
Copy link
Contributor

I suggest putting docs and tests in the same commit in the future, it makes them "obviously correct".

But LGTM as is.

@jasnell
Copy link
Member

jasnell commented Dec 29, 2016 •
edited
Loading

This is not landing cleanly in master despite this not showing any conflicts here. Can you rebase and update the PR nevermind! appears to have been a temporary glitch on my end

jasnell pushed a commit that referenced this pull request Jan 6, 2017
@thefourtheye @jasnell
doc: killSignal option accepts integer values
fc647fd 
`killSignal` option accepts the signal name or signal number as well.

PR-URL: #10424
Reviewed-By: Julian Duque <julianduquej@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
@jasnell
Copy link
Member

jasnell commented Jan 6, 2017

Landed in fc647fd

@jasnell jasnell closed this Jan 6, 2017
@thefourtheye thefourtheye deleted the doc-killsignal-type-update branch January 7, 2017 00:51
italoacasas pushed a commit to italoacasas/node that referenced this pull request Jan 18, 2017
@thefourtheye @italoacasas
doc: killSignal option accepts integer values
14870d4 
`killSignal` option accepts the signal name or signal number as well.

PR-URL: nodejs#10424
Reviewed-By: Julian Duque <julianduquej@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
italoacasas pushed a commit to italoacasas/node that referenced this pull request Jan 19, 2017
@thefourtheye @italoacasas
doc: killSignal option accepts integer values
1aa359e 
`killSignal` option accepts the signal name or signal number as well.

PR-URL: nodejs#10424
Reviewed-By: Julian Duque <julianduquej@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
italoacasas pushed a commit to italoacasas/node that referenced this pull request Jan 24, 2017
@thefourtheye @italoacasas
doc: killSignal option accepts integer values
add407f 
`killSignal` option accepts the signal name or signal number as well.

PR-URL: nodejs#10424
Reviewed-By: Julian Duque <julianduquej@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
@italoacasas italoacasas mentioned this pull request Jan 25, 2017
Closed
italoacasas pushed a commit to italoacasas/node that referenced this pull request Jan 27, 2017
@thefourtheye @italoacasas
doc: killSignal option accepts integer values
8314d9e 
`killSignal` option accepts the signal name or signal number as well.

PR-URL: nodejs#10424
Reviewed-By: Julian Duque <julianduquej@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
@italoacasas italoacasas mentioned this pull request Jan 29, 2017
Merged
thefourtheye added a commit to thefourtheye/io.js that referenced this pull request Feb 5, 2017
@thefourtheye
doc: killSignal option accepts integer values
f90a4f4 
`killSignal` option accepts the signal name or signal number as well.

PR-URL: nodejs#10424
Reviewed-By: Julian Duque <julianduquej@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
@thefourtheye thefourtheye mentioned this pull request Feb 5, 2017
Closed
2 tasks
thefourtheye added a commit to thefourtheye/io.js that referenced this pull request Feb 5, 2017
@thefourtheye
doc: killSignal option accepts integer values
79af25f 
`killSignal` option accepts the signal name or signal number as well.

PR-URL: nodejs#10424
Reviewed-By: Julian Duque <julianduquej@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
@thefourtheye thefourtheye mentioned this pull request Feb 5, 2017
Closed
2 tasks
MylesBorins pushed a commit that referenced this pull request Feb 6, 2017
@thefourtheye @MylesBorins
doc: killSignal option accepts integer values
8eeccd8 
`killSignal` option accepts the signal name or signal number as well.

PR-URL: #10424
Reviewed-By: Julian Duque <julianduquej@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Feb 6, 2017
Merged
MylesBorins pushed a commit that referenced this pull request Feb 21, 2017
@thefourtheye @MylesBorins
doc: killSignal option accepts integer values
8c784eb 
`killSignal` option accepts the signal name or signal number as well.

PR-URL: #10424
Reviewed-By: Julian Duque <julianduquej@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
MylesBorins pushed a commit that referenced this pull request Feb 21, 2017
@thefourtheye @MylesBorins
doc: killSignal option accepts integer values
78b83e7 
`killSignal` option accepts the signal name or signal number as well.

PR-URL: #10424
Reviewed-By: Julian Duque <julianduquej@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@julianduque julianduque julianduque approved these changes

@jasnell jasnell jasnell approved these changes

Assignees
No one assigned
Labels
child_process Issues and PRs related to the child_process subsystem. doc Issues and PRs related to the documentations.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@thefourtheye @sam-github @jasnell @julianduque @MylesBorins @nodejs-github-bot